Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't detect copies/renames in set-parameters #103

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

benjaminjkraft
Copy link

The big benefit of this is in a blobless clone, where it lets you avoid downloading all the blobs (which you shouldn't need just to know whether a file changed). But it's probably a perf win always: it makes git diff do a bit less work. And it's probably more correct: for the purposes of whether to run a CI job, a move/copy should count as a modification -- for example one should assume the file must be rebuilt in its new location, the tests for both old and new location should be run, etc.

I tested this by running

git diff --name-only HEAD HEAD~1000 >/dev/null

in a blobless clone of a large repo. In that case it logs some messages about the fetches it's doing. (And empirically in our actual CI pipeline it has occasionally failed, if the blob fetch fails due to network flakes.) With the new flags, it does not, and returns much more quickly.

The big benefit of this is in a [blobless clone], where it lets you
avoid downloading all the blobs (which you shouldn't need just to know
_whether_ a file changed). But it's probably a perf win always: it makes
`git diff` do a bit less work. And it's probably more correct: for the
purposes of whether to run a CI job, a move/copy should count as a
modification -- for example one should assume the file must
be rebuilt in its new location, the tests for both old and new location
should be run, etc.

[blobless clone]: https://github.blog/2020-12-21-get-up-to-speed-with-partial-clone-and-shallow-clone/

I tested this by running
```sh
git diff --name-only HEAD HEAD~1000 >/dev/null
```
in a blobless clone of a large repo. In that case it logs some messages
about the fetches it's doing. (And empirically in our actual CI pipeline
it has occasionally failed, if the blob fetch fails due to network
flakes.) With the new flags, it does not, and returns much more quickly.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant